Skip to content

Add retry support for HTTP execution failure#1702

Merged
balamurugana merged 2 commits into
minio:masterfrom
balamurugana:Add-retry-support-for-HTTP-execution-failure
May 14, 2026
Merged

Add retry support for HTTP execution failure#1702
balamurugana merged 2 commits into
minio:masterfrom
balamurugana:Add-retry-support-for-HTTP-execution-failure

Conversation

@balamurugana
Copy link
Copy Markdown
Member

No description provided.

@balamurugana balamurugana marked this pull request as draft May 11, 2026 15:13
@balamurugana balamurugana force-pushed the Add-retry-support-for-HTTP-execution-failure branch 8 times, most recently from 5475995 to 81ad96c Compare May 12, 2026 08:11
@balamurugana balamurugana force-pushed the Add-retry-support-for-HTTP-execution-failure branch from 81ad96c to 9e8f51f Compare May 12, 2026 08:25
@balamurugana balamurugana marked this pull request as ready for review May 12, 2026 08:25
Comment thread api/src/main/java/io/minio/Http.java
shtripat
shtripat previously approved these changes May 12, 2026
Copy link
Copy Markdown

@shtripat shtripat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Copy link
Copy Markdown
Contributor

@allanrogerr allanrogerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new retry logic including interceptor, setRetry replacement logic, body-replay across retries, NPE-safe trace formatting are not unit-tested. The functional tests cover a happy path, but

  • no test creates a scenario to verify retry actually fires,
  • no test verifies the maxRetries boundary,
  • no test checks setRetry replaces the interceptor correctly,
  • no test exercises a malformed request.method()
    Could you have Claude generate some tests please?

Comment thread api/src/main/java/io/minio/Http.java Outdated
Comment thread api/src/main/java/io/minio/BaseS3Client.java Outdated
Comment thread api/src/main/java/io/minio/BaseS3Client.java
Comment thread api/src/main/java/io/minio/BaseS3Client.java Outdated
Copy link
Copy Markdown
Contributor

@allanrogerr allanrogerr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional comments @balamurugana PTAL

Comment thread api/src/main/java/io/minio/BaseS3Client.java Outdated
Comment thread api/src/main/java/io/minio/BaseS3Client.java
Comment thread api/src/main/java/io/minio/Http.java
@balamurugana balamurugana force-pushed the Add-retry-support-for-HTTP-execution-failure branch 3 times, most recently from 6afbeba to 9b53fa8 Compare May 13, 2026 02:41
@allanrogerr
Copy link
Copy Markdown
Contributor

Rechecking

Comment thread api/src/test/java/io/minio/MinioClientTest.java
Comment thread api/src/main/java/io/minio/Http.java
Comment thread api/src/main/java/io/minio/Http.java
@allanrogerr
Copy link
Copy Markdown
Contributor

Just a couple missing tests if you dont mind and an NPE guard. Thank you for this solution @balamurugana !

Signed-off-by: Bala.FA <bala@minio.io>
@balamurugana balamurugana force-pushed the Add-retry-support-for-HTTP-execution-failure branch from 9b53fa8 to 248990b Compare May 13, 2026 13:24
@balamurugana
Copy link
Copy Markdown
Member Author

Just a couple missing tests if you dont mind and an NPE guard. Thank you for this solution @balamurugana !

@allanrogerr Updated the PR. Thanks!

@allanrogerr
Copy link
Copy Markdown
Contributor

@shtripat @rraulinio PTAL

Copy link
Copy Markdown

@shtripat shtripat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@balamurugana balamurugana merged commit 7790791 into minio:master May 14, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants